Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Activate TypeScript #75

Merged
merged 9 commits into from
May 8, 2019
Merged

Activate TypeScript #75

merged 9 commits into from
May 8, 2019

Conversation

Vinnl
Copy link
Contributor

@Vinnl Vinnl commented May 7, 2019

(Note: only relevant after #74 is merged.)

This PR enables type checking. The approach is as follows:

  • There's a Babel plugin to remove type annotations from the code.
  • In parallel to compiling the code, Fork-TS-Checker-Plugin (for Webpack) checks the type annotations.

I chose this approach because it appears we're already using Babel, and this makes it easy not to have type checking block compilation, which helps with gradual adoption.

TypeScript is currently set to strict mode, but is only active for .ts files. This means we can gradually migrate by renaming files, and simply disable type checking for specific values using any when it's too much work to introduce proper types.

Critique away! :)

Fixes #69.

Vinnl added 4 commits May 7, 2019 16:04
package-lock.json has been generated with version 6.6 of npm (or
higher). In this version, npm updated the format, so to avoid
changes being done and undone to package-lock.json, we now tell
tools like Node version manager that we'd prefer a recent version
of Node.

See
https://stackoverflow.com/questions/54411377/package-lock-json-file-package-with-optional-true
Although package.json referenced Webpack, there was not actually a
configuration file for Webpack, and it could not be run
successfully. It appears that the code was published as-is.

I've now added a configuration file to pass the code through Babel,
using the current version of Babel. The module is still exported as
a default CommonJS module, but is now located in dist/main.js
instead of index.js.
This allows files to be written with a `.ts` extension, and for
actual TypeScript syntax to be used inside those files.

Note, though, that at this point the types are not actually
actively checked (unless by your editor). All this does is
stripping the type definitions and parsing the .ts files as
Javascript.
This activates type checking, failing the build if a .ts file does
not pass type checking. Strict mode is currently set to true,
which appears to be workable somewhat by only converting single
files to .ts, and then e.g. adding explicit `any` annotations in
places where it's currently too much work to add explicit types.
@megoth megoth changed the base branch from master to revamp May 8, 2019 16:58
Copy link
Contributor

@megoth megoth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general I want to avoid type unknown as much as possible, but will allow it for this PR.

Vinnl and others added 4 commits May 8, 2019 19:01
This allows files to be written with a `.ts` extension, and for
actual TypeScript syntax to be used inside those files.

Note, though, that at this point the types are not actually
actively checked (unless by your editor). All this does is
stripping the type definitions and parsing the .ts files as
Javascript.
This activates type checking, failing the build if a .ts file does
not pass type checking. Strict mode is currently set to true,
which appears to be workable somewhat by only converting single
files to .ts, and then e.g. adding explicit `any` annotations in
places where it's currently too much work to add explicit types.
@megoth megoth merged commit 3235089 into revamp May 8, 2019
@Vinnl
Copy link
Contributor Author

Vinnl commented May 9, 2019

@megoth They're basically there to indicate "do not forget to properly type this later". Since they cannot be used until they get properly typed anyway, I figured this is a way to avoid typing values that are currently unused (hence speeding up the migration), especially considering that I do not know what the proper interface would be :) (See also the PaneDefinition interface, which I think is probably mostly correct, but is still the result of me guessing the interface based on other code and hence might not be entirely correct.)

In other words: I'd be happy for you to insert the correct types if you know them, but given that an incorrect type is probably worse than no type, I left it at this :)

@timea-solid timea-solid deleted the babel-typescript branch February 28, 2022 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants